Reduce cache footprint by decoupling degeneracy-dependent data#387
Reduce cache footprint by decoupling degeneracy-dependent data#387
Conversation
Codecov Report❌ Patch coverage is
... and 20 files with indirect coverage changes 🚀 New features to boost your workflow:
|
| Base.parent(h::Hashed) = h.val | ||
|
|
||
| # hash overload | ||
| Base.hash(h::Hashed{T, Hash}, seed::UInt) where {T, Hash} = Hash(parent(h), seed) |
There was a problem hiding this comment.
I find it a bit confusing if type parameters encode a function, that they still use capital letters. I was looking for a type called Hash, before realising that this is actually the custom hash function encoded in the type. I understand that using hash for the type parameter is out of the question.
A related question is whether you know if there is any benefits or downsides of this design compared to using the function as a field, i.e.
struct Hashed{T, H, E}
val::T
hashf::H
eqf::E
endand then doing things like h.hashf(parent(h), seed). I am asking because that is how InnerProductVec over at KrylovKit works, and if it has downsides (other than the stylistic differences), I should probably change this.
There was a problem hiding this comment.
I actually don't think it does, if these are singleton types (like Function) the size remains the same, so AFAIK this is equivalent and just a style choice. I'll refactor along with the required rebase :)
| @@ -0,0 +1,276 @@ | |||
| # Block and fusion tree ranges: structure information for building tensors | |||
| #-------------------------------------------------------------------------- | |||
There was a problem hiding this comment.
Even though some of this information is very much tailored towards the tensor construction, in my mind spaces come before tensors, and could in principle exist on their own. However, the current code in the spaces subfolder is broken without the definitions in this file, so I would prefer this file to live in the spaces folder. (There is not a single mention of TensorMap in this file).
| end | ||
| for (w₁, w₂) in zip(domain(W₁), domain(W₂)) | ||
| isdual(w₁) == isdual(w₂) || return false | ||
| isequal(sectors(w₁), sectors(w₂)) || return false |
There was a problem hiding this comment.
I don't think the return value of sectors was really intended for direct comparison:
julia> isequal(sectors(SU2Space(0=>2)), sectors(SU2Space(0=>2)))
falseThere was a problem hiding this comment.
Maybe isempty(symdiff(sectors(w₁), sectors(w₂))) ?
| bs = Indices{I}() | ||
| for s in sectors(P), c in ⊗(s...) | ||
| set!(bs, c) |
There was a problem hiding this comment.
I don't think this covers the N==0 case, and is also overly complicated for the N==1 case.
| for c in _blocksectors(W) | ||
| push!(bs, c) | ||
| codom_start = length(trees) + 1 | ||
| n₁ = 0 | ||
| for f₂ in fusiontrees(dom, c) | ||
| if n₁ == 0 | ||
| # First f₂ for this sector: enumerate codomain trees and record how many there are. | ||
| for f₁ in fusiontrees(codom, c) | ||
| push!(trees, (f₁, f₂)) | ||
| end | ||
| n₁ = length(trees) - codom_start + 1 | ||
| else | ||
| # Subsequent f₂s: the codomain trees are already in the list at | ||
| # codom_start:codom_start+n₁-1, so read them back instead of recomputing. | ||
| for j in codom_start:(codom_start + n₁ - 1) | ||
| push!(trees, (trees[j][1], f₂)) | ||
| end | ||
| end | ||
| end | ||
| end |
There was a problem hiding this comment.
I'd say there are quite a few unnecessary +1 and -1s because of your choice of using the starting index, instead of the offset:
| for c in _blocksectors(W) | |
| push!(bs, c) | |
| codom_start = length(trees) + 1 | |
| n₁ = 0 | |
| for f₂ in fusiontrees(dom, c) | |
| if n₁ == 0 | |
| # First f₂ for this sector: enumerate codomain trees and record how many there are. | |
| for f₁ in fusiontrees(codom, c) | |
| push!(trees, (f₁, f₂)) | |
| end | |
| n₁ = length(trees) - codom_start + 1 | |
| else | |
| # Subsequent f₂s: the codomain trees are already in the list at | |
| # codom_start:codom_start+n₁-1, so read them back instead of recomputing. | |
| for j in codom_start:(codom_start + n₁ - 1) | |
| push!(trees, (trees[j][1], f₂)) | |
| end | |
| end | |
| end | |
| end | |
| for c in _blocksectors(W) | |
| push!(bs, c) | |
| offset = length(trees) | |
| n₁ = 0 | |
| for f₂ in fusiontrees(dom, c) | |
| if n₁ == 0 | |
| # First f₂ for this sector: enumerate codomain trees and record how many there are. | |
| for f₁ in fusiontrees(codom, c) | |
| push!(trees, (f₁, f₂)) | |
| end | |
| n₁ = length(trees) - offset | |
| else | |
| # Subsequent f₂s: the codomain trees are already in the list at | |
| # offset .+ (1:n₁), so read them back instead of recomputing. | |
| for j in offset .+ (1:n₁) | |
| push!(trees, (trees[j][1], f₂)) | |
| end | |
| end | |
| end | |
| end |
2 participants
Refactors the internal block structure representation for
TensorMapby splittingFusionBlockStructureinto two separately-cached structs:SectorStructure(cached by sector structure, shared across spaces with the same sectors): coupled sectors and valid fusion tree pairs asDictionaries.Indices.DegeneracyStructure(cached perHomSpace): total dimension, block sizes/ranges, and sub-block strides as plainVectors.The public
blockstructure(W)andsubblockstructure(W)combine these on the fly into aDictionary.Previously,
FusionBlockStructurestored afusiontreelist, afusiontreeindiceslookup dict, and afusiontreestructurevector — the tree pairs were stored twice and lookup required manual indirection.Dictionaries.jlhandles this naturally via its token system.A secondary benefit:
DegeneracyStructure{N}is parameterized only by the number of indicesN, not the sector type. Kernels that only need sizes/strides/offsets can therefore be compiled once and reused across different symmetry groups — a potential route to much faster precompilation.Also moves the tensor structure computation out of
homspace.jlinto a newtensorstructure.jl.Open questions:
Dictionaries.jltypes to avoid maintaining two APIs?